-
Notifications
You must be signed in to change notification settings - Fork 25.5k
Add convenience API key param to remote reindex #135949
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add convenience API key param to remote reindex #135949
Conversation
Hi @PeteGillinElastic, I've created a changelog YAML for you. |
🔍 Preview links for changed docs |
ℹ️ Important: Docs version tagging👋 Thanks for updating the docs! Just a friendly reminder that our docs are now cumulative. This means all 9.x versions are documented on the same page and published off of the main branch, instead of creating separate pages for each minor version. We use applies_to tags to mark version-specific features and changes. Expand for a quick overviewWhen to use applies_to tags:✅ At the page level to indicate which products/deployments the content applies to (mandatory) What NOT to do:❌ Don't remove or replace information that applies to an older version 🤔 Need help?
|
This adds a `source.remote.api_key` parameter to the reindex API. This is equivalent to using `source.remote.headers` with `Authorization` set to `ApiKey <api_key>`. It is a convenience for the user. Note on docs changes: The example request using an API key now uses an `applies-switch` to show both the pre-9.3 and post-9.3 / serverless versions. There are a few drive-by improvements to the docs, including: - Adding some subsections, as suggested by a tech writer. - Fixing the bug which meant that `<OTHER_HOST_URL>` was previously not being rendered, because `<` is a control character. - Repeating the note about using HTTPS for basic auth in the API key section, as we wouldn't recommend sending API keys over plain HTTP either. - A nit, but writing `<OTHER_HOST_URL>:9200` is strange, since we have a placeholder URL, and the port is part of the URL, so that's fixed.
e26d4db
to
5af6d91
Compare
Relevant deep-link into the docs preview: https://docs-v3-preview.elastic.dev/elastic/elasticsearch/pull/135949/reference/elasticsearch/rest-apis/reindex-indices#reindex-from-remote |
Pinging @elastic/es-data-management (Team:Data Management) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds a convenient source.remote.api_key
parameter to the reindex API as an alternative to manually setting the Authorization header through source.remote.headers
. The change simplifies API key authentication for remote reindexing operations.
- Added a convenience
api_key
parameter that automatically sets the Authorization header toApiKey <api_key>
- Enhanced documentation with better structure, fixed rendering issues, and added version-specific examples
- Added comprehensive test coverage for the new API key functionality
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
ReindexRequest.java | Implements the core logic for processing the api_key parameter and validating conflicts with existing Authorization headers |
ReindexRequestTests.java | Adds test cases for api_key functionality including conflict detection and header merging scenarios |
reindex-indices.md | Updates documentation with improved structure, fixed HTML encoding issues, and version-specific examples using applies-switch |
135949.yaml | Documents the enhancement in the changelog |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
if (original.keySet().stream().anyMatch(key -> key.equalsIgnoreCase("Authorization"))) { | ||
throw new IllegalArgumentException("Cannot specify both [api_key] and [headers] including [Authorization] key"); | ||
} | ||
Map<String, String> updated = (original instanceof HashMap) ? original : new HashMap<>(original); |
Copilot
AI
Oct 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic mutates the original HashMap which could lead to unexpected side effects. Consider always creating a new HashMap to maintain immutability and avoid modifying the caller's data structure.
Map<String, String> updated = (original instanceof HashMap) ? original : new HashMap<>(original); | |
Map<String, String> updated = new HashMap<>(original); |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can see why you have done this here to conserve memory when the update isn't required. Apparently CoPilot didn't pick up on that nuance...
"host": "<OTHER_HOST_URL>", | ||
"api_key": "<API_KEY_VALUE>" |
Copilot
AI
Oct 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The HTML entity encoding <
appears to be incorrectly used for documentation placeholders. This should likely be <OTHER_HOST_URL>
or properly escaped for the documentation format being used.
"host": "<OTHER_HOST_URL>", | |
"api_key": "<API_KEY_VALUE>" | |
"host": "<OTHER_HOST_URL>", | |
"api_key": "<API_KEY_VALUE>" |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not 100% sure what's right here. I can foresee the docs getting hung up thinking this is an HTML tag if using an actual <
but also can imagine there being escaping logic that might have this render out as a literal <
.
I'm guessing you have tested it out / copied previous examples?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I tested this locally. I recommend doing that for docs changes where you're not completely confident, by the way. Running the v3 tool is fairly easy once you've got it installed, and it does some kind of hot reloading so the feedback loop is very fast if you want to experiment.
You can also you can see the preview here: https://docs-v3-preview.elastic.dev/elastic/elasticsearch/pull/135949/reference/elasticsearch/rest-apis/reindex-indices#reindex-from-remote .
Interestingly, it doesn't like it if you replace the >
with &rt;
, which I was going to do for consistency.
I happened to be discussing this in a thread with the docs team (because I needed advice on the version thing) so I asked there whether there's some other option that's preferred. I'll give them a little while to respond, and if they don't then I'll merge it like this, since it does work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍🏻 Nice bit of UX improvement
This reflects the change made in elastic/elasticsearch#135949.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for the improvements / bug fixes here. added a couple of comments to take or leave.
is this feature releasing immediately for serverless? if it isn't, you might have to separate your docs and code changes so you can add the docs when the feature becomes available.
|
||
### Using an API key [reindex-api-key] | ||
|
||
When using {{ecloud}}, it is also possible (and encouraged) to authenticate with the remote cluster through the use of a valid API key: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this ECH, or does this also work for ECE / ECK?
When using {{ecloud}}, it is also possible (and encouraged) to authenticate with the remote cluster through the use of a valid API key: | |
When using {{ech}}, you can also authenticate with the remote cluster through the use of a valid API key. This is the recommended method. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It refers to the keys described here: https://www.elastic.co/docs/deploy-manage/api-keys/elasticsearch-api-keys . Actually, I think it also works against serverless with the keys described here: https://www.elastic.co/docs/deploy-manage/api-keys/serverless-project-api-keys although I haven't tested that. (FYI, I didn't write this text, even though the github diff makes it look kind of like I did...)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that makes me think that the keys would work for any deployment type. is there any reason from a code pov that these would be limited by deployment type? stuff at the stack level is mostly extra limited in cloud rather than the opposite. we should make sure, but i'd probably take off the "when using elastic cloud" caveat.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've tested it with a local dev instance where I created a key via the POST _security/api_key
API, and it worked fine. So I will go ahead and remove the caveat, as you suggest.
:::: | ||
|
||
|
||
Be sure to use `https` when using an API key, or it will be sent in plain text. There are a range of settings available to configure the behaviour of the `https` connection. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where are these settings? could downlink to the anchors if they're below. do those settings apply to basic auth?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, the settings referenced here are the generic SSL settings, they're nothing to do with the auth method used. (This paragraph is a copy-and-paste from the existing text in the basic auth section, because I figured that the advice applied equally to both cases.) I've pushed a commit with the cross-reference.
Co-authored-by: shainaraskas <58563081+shainaraskas@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @shainaraskas . I've taken one suggestion, addressed another comment, there's one comment where I answered your question and await your advice (around the cloud versions).
|
||
### Using an API key [reindex-api-key] | ||
|
||
When using {{ecloud}}, it is also possible (and encouraged) to authenticate with the remote cluster through the use of a valid API key: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It refers to the keys described here: https://www.elastic.co/docs/deploy-manage/api-keys/elasticsearch-api-keys . Actually, I think it also works against serverless with the keys described here: https://www.elastic.co/docs/deploy-manage/api-keys/serverless-project-api-keys although I haven't tested that. (FYI, I didn't write this text, even though the github diff makes it look kind of like I did...)
:::: | ||
|
||
|
||
Be sure to use `https` when using an API key, or it will be sent in plain text. There are a range of settings available to configure the behaviour of the `https` connection. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, the settings referenced here are the generic SSL settings, they're nothing to do with the auth method used. (This paragraph is a copy-and-paste from the existing text in the basic auth section, because I figured that the advice applied equally to both cases.) I've pushed a commit with the cross-reference.
|
||
::::{applies-switch} | ||
|
||
:::{applies-item} { "stack": "ga 9.3", "serverless": } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't know about this stack version annotation, I guess it runs the below command against the specified stack and version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we don't have version-specific docs as of 9.x, this makes the page show a selector where the reader can pick the appropriate tab and it shows the relevant snippet. You can see that in the preview.
|
||
|
||
Be sure to use `https` when using an API key, or it will be sent in plain text. There are a range of settings available to configure the behaviour of the `https` connection. | ||
Be sure to use `https` when using an API key, or it will be sent in plain text. There are a [range of settings](#reindex-ssl) available to configure the behaviour of the `https` connection. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
…y section, since you can use API keys on pretty much any ES type
* Add `api_key` param to remote reindex This reflects the change made in elastic/elasticsearch#135949. * Update specification/_global/reindex/types.ts Co-authored-by: Quentin Pradet <quentin.pradet@elastic.co> * Also code-font the `Authorization` header name * make contrib --------- Co-authored-by: Quentin Pradet <quentin.pradet@elastic.co>
This adds a
source.remote.api_key
parameter to the reindex API. This is equivalent to usingsource.remote.headers
withAuthorization
set toApiKey <api_key>
. It is a convenience for the user.Note on docs changes: The example request using an API key now uses an
applies-switch
to show both the pre-9.3 and post-9.3 / serverless versions. There are a few drive-by doc bug-fixes and other improvements to the docs, most notably:Adding some subsections, as suggested by a tech writer.
Fixing the bug which meant that
<OTHER_HOST_URL>
was previously not being rendered, because<
is a control character.Repeating the note about using HTTPS for basic auth in the API key section, as we wouldn't recommend sending API keys over plain HTTP either.
A nit, but writing
<OTHER_HOST_URL>:9200
is strange, since we have a placeholder URL, and the port is part of the URL, so that's fixed.